Skip to content

feat(hooks): defineHook — typed hook authoring with ctx payload/wrap/abort - #6100

Draft
edusperoni wants to merge 16 commits into
mainfrom
feat/define-hook
Draft

feat(hooks): defineHook — typed hook authoring with ctx payload/wrap/abort#6100
edusperoni wants to merge 16 commits into
mainfrom
feat/define-hook

Conversation

@edusperoni

Copy link
Copy Markdown
Collaborator

Stacked on #6099 (feat/di-modernization-phase1).

PR Checklist

What is the current behavior?

Hooks are plain functions whose shape the CLI infers at runtime: services arrive via parameter-name injection (deprecated, runtime-traced), the payload via a magic hookArgs parameter, middleware by returning a function (implicit and undocumented), and aborting by throwing an error carrying stopExecution/errorAsWarning fields.

What is the new behavior?

A typed, explicit hook-authoring API — fully additive; every existing hook keeps working unchanged.

const { defineHook, inject, DoctorService } = require("nativescript/contracts");

module.exports = defineHook("before-prepare", async (ctx) => {
	const doctorService = inject(DoctorService);   // services via inject(), as everywhere else
	ctx.payload.args.push("--offline");            // the operation payload, mutable as before
	ctx.wrap(async (args, next) => next(...args)); // explicit middleware (was: return a function)
	ctx.abort("reason", { asWarning: true });      // explicit abort (was: throw + stopExecution)
});
  • lib/common/define-hook.ts is import-free (loading it can never boot a second CLI runtime) and re-exported from nativescript/contracts; definitions carry a Symbol.for marker so duplicated CLI copies in an extension tree recognize each other's definitions.
  • hooks-service runs definition modules on a dedicated path: no parameter-name parsing, no projectData promotion hack, no deprecation report. Plain-function hooks are untouched. .mjs default exports are recognized.
  • wrap() middlewares feed the exact channel legacy returned-functions use, so they compose with the @hook decorator chain identically (11 new tests pin payload identity/mutation, inject-in-handler, middleware wrap + short-circuit, abort both ways, legacy coexistence + tracer isolation, .mjs, and name-mismatch tracing).
  • One deliberate edge: per-directory hook results are now flattened once so multi-middleware hooks surface correctly. A legacy hook that returned an array of functions was silently dropped before and becomes middleware now — returning anything was always outside the documented contract (fulfillment values are ignored), so this is considered theoretical.
  • extending-cli.md now leads with defineHook; plain-function hooks and parameter-name injection remain documented as the transitional and legacy tiers.

Full suite: 110 files, 1609 passed / 38 skipped; the yok oracle, public-API test, and compat fixtures are untouched.

Purpose-built container with an Angular-compatible surface (inject,
runInInjectionContext, Injector, provide/provideLazy, forwardRef).
Lookup is class-object first with a fallback to the decorator-set name,
per injector level, so per-call string overrides and duplicated contract
copies in the extensions tree resolve to the same provider. Includes a
legacy provider kind that constructs Yok-style classes via annotate(),
lazy side-effect loaders for path-based registration, transient
retention, and reverse-instantiation-order disposal.
reportDeprecation() dedups per api+detail and logs at trace level by
default; NS_DEPRECATIONS=warn|error previews the stricter stages so the
same call sites can be escalated over releases. Wired at the external
entry points only: param-name hook invocation, require-time extension
registration, and dynamicCall help templating.
A class migrated off property injection has neither $hooksService nor
$injector, so the decorator threw at hook-execution time. The global
injector must stay last in the chain - tests stub the instance
properties and rely on them winning.
Yok keeps its entire public surface, subclassability, and the global
$injector, while storage and resolution delegate to the new container.
Command routing, key commands, and the public-API builder are unchanged.
Every legacy member now carries @deprecated JSDoc naming its
replacement, mirrored on IInjector.
DoctorService and ProjectNameService become @contract abstract classes;
their impls are renamed *Impl (externally invisible - outside resolution
is by string name or token, never class identity). The subpath resolves
through contracts/package.json rather than an exports map, so existing
deep requires keep working, and the entry point is side-effect-free so a
duplicated CLI copy in an extensions tree never boots a second runtime.
Fixtures exercise the surfaces third parties rely on: param-name hook
signatures across every payload shape and influence channel (mutation,
function-return middleware, abort), require-time extension registration
against global.$injector including hierarchical commands, and the full
IInjector facade surface. Notably they pin that a hook naming an
unwrapped payload key (the after-watchAction shape) is skipped as
invalid - resolution changes must not resurrect long-dead hooks.
dependency-injection.md covers tokens (@contract), inject()/Injector,
provider kinds, resolution semantics, forwardRef, coexistence with the
legacy $injector, and a legacy-to-new quick reference. extending-cli.md
leads with the recommended hook pattern - inject() works directly in
hook bodies because they run in an injection context, pinned by a new
compat test - and demotes parameter-name injection to a labeled legacy
section. String-token lookups are framed as the migration bridge, not a
co-equal API.

The hook deprecation tracer now flags only hooks that actually use
param-name service injection; a hookArgs-only signature follows the
recommended pattern and stays silent.
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 287a7505-c08f-45cf-a659-2535538e6a9b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

createHierarchicalCommand registered the synthesized parent - and its
execute path resolved commandsService and errors - through the
module-level global injector instead of the instance it was called on,
so a parent dispatcher leaked onto the global injector whenever a
hierarchical command was registered on any other instance.

The require-ordering guard accidentally depended on that leak: with the
parent now landing on the same instance, it is exempted for synthesized
parents (they exist because a child registered, not because requires ran
out of order), and the else branch mirrors the existing default-command
skip. Net effect: register-then-require orderings that used to throw
'Default commands should be required before child commands' now work;
nothing that worked before changes.
Internal code no longer reads global.$injector: the two cycle-bound
sites (the deprecation tracer's logger fallback and @hook's last-resort
lookup) go through a getInjector() accessor required at call time. The
global property remains write-only from the CLI's side - it exists as
the published surface for extensions and hooks.
The token container was reachable only through a getter on the concrete
Yok class, so every crossing from facade-typed code into the new API
needed an any cast - the migration's most important seam was invisible
to the type system. IInjector now declares readonly di: Injector.
Yok is now an Injector - class Yok extends Injector - so the new API
works on the facade directly (get, register with Providers, createChild,
runInInjectionContext($injector, ...)) and inject(Injector) inside
legacy-constructed classes returns the facade itself instead of a
second, inner container identity. The di bridge is gone. register()
dispatches by argument shape: a string first argument is the legacy
name-based form, anything else is a Provider.

IInjector now extends the Injector class type, which constrains
implementers to the real class hierarchy (Yok and its subclasses) -
intentional, since the interface only ever described Yok.
IInjector recomposes from per-subsystem faces - CommandRegistry,
KeyCommandRegistry, ModuleRegistry, PublicApiBuilder - each an @contract
token the facade registers itself under. One object still implements
everything until the subsystems are physically extracted; extraction
then becomes a provider swap for the face's token instead of a consumer
migration. Consumers can depend on the narrow face they actually use,
and deprecation becomes per-face instead of a flat everything-is-legacy.

The contracts are internal (lib/common/contracts) and deliberately not
re-exported from nativescript/contracts; promoting one is a per-contract
decision.
- doctor-service: printWarnings accepts an optional trackResult matching
  its contracts; runSetupScript returns the setup script result instead
  of resolving undefined; canExecuteLocalBuild no longer dereferences
  its optional argument
- deprecation tracer: a report dropped for lack of a logger is no
  longer latched as delivered - it reports once a logger exists
- yok: global.$injector is an accessor pair, so a direct third-party
  assignment stays synchronized with the binding getInjector() reads
- docs: note the useValue + shared:false quirk; show where Injector is
  imported from in the late-lookup example
optional resolves to null instead of throwing for unknown tokens (a
found-but-misconfigured provider still throws); skipSelf starts at the
parent, escaping a child scope's shadowing entry; self refuses parent
fallthrough. self+skipSelf throws. host is deliberately absent - it is
an Angular component-tree concept with no analog in this hierarchy.

get()'s former second parameter - the legacy per-call ctorArguments bag
- had exactly one caller, the facade's own resolve(name, bag). It moves
to a protected getWithLegacyArguments channel, so the public get()
aligns with Angular's shape today rather than after the legacy paths
are deleted.
Hook authors can now export a definition built with defineHook instead
of a plain function whose shape the CLI has to infer. The handler takes
a context object with the operation payload, an explicit wrap() for
middleware around the hooked method, and abort() for stopping the hook
as either a failure or a warning.

Definitions are marked with Symbol.for("nativescript:cli:hookDefinition")
so a duplicated CLI copy in an extension's dependency tree still
recognizes them. The definition path skips parameter-name resolution,
the projectData promotion hack and the deprecation report; plain
function hooks keep running through the existing path unchanged.

lib/common/define-hook.ts is import-free so a hook can load it without
booting a second runtime, and it is re-exported from
nativescript/contracts.
Yok extends Injector on the base branch, so definitions run in
runInInjectionContext(this.$injector, ...) directly and
inject(Injector) inside a hook returns the facade itself.
Base automatically changed from feat/di-modernization-phase1 to main August 3, 2026 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant